Conversation
romainguy
left a comment
romainguy
left a comment
There was a problem hiding this comment.
How well does this interact with luminance scaling? Can you try? If it doesn't behave well (since AgX kind of does it already) we may want to no-op luminanceScaling with AgX.
filament/src/ToneMapper.cpp
Outdated
| } | ||
|
|
||
| float3 AgxToneMapper::operator()(float3 v) const noexcept { | ||
| // TODO: It's unclear if we need to temporarily transform to 709 primaries again. |
There was a problem hiding this comment.
If you don't change ColorGrading.cpp the values you'll get here are in Rec2020. See selectColorGradingTransformIn()/selectColorGradingTransformOut(). Seems like if we don't do anything we'll match Blender? Might be worth checking Blender's implementation to see what they do.
There was a problem hiding this comment.
Right, which I why I originally converted back to sRGB with the commented-out: v = Rec2020_to_sRGB * v;. The way I have it matches Blender (and looks good to me), but I'm not sure if it's right, as Sobotka's original AgX seems to keep everything in 709.
There was a problem hiding this comment.
I can put you in touch with Troy on Discord if you want. Does Blender maybe use a different inset/outset matrix? The idea is to just slightly expand/contract the gamut.
There was a problem hiding this comment.
Technically yes, Blender's uses different primaries_rotate and primaries_scale parameters, resulting in a different inset/outset matrices. I'm not sure if they adjusted the parameters for aesthetic reasons or to somehow compensate for Rec2020.
There was a problem hiding this comment.
I would expect it was made to map better to Rec2020. That said we don't render in 2020, we only use 2020 during color grading.
output.mp4I wouldn't say it breaks it, but I do I think I prefer AgX without luminance scaling. |
|
Agreed, it's better without, but prob not worth no-oping luminance scaling when AgX is selected. |
romainguy
left a comment
romainguy
left a comment
There was a problem hiding this comment.
LGTM so far, let me know what agxLook() does. Might be something optional driven by a parameter?
FYI looks.mp4 |
| } | ||
|
|
||
| // ASC CDL | ||
| val = pow(val * slope + offset, power); |
There was a problem hiding this comment.
Note: our existing color grading APIs let you apply a CDL.
3 participants
AgX:
ACES (legacy):
